feat(core): Expose previous entity state on CollectionEvent updates - #5067
feat(core): Expose previous entity state on CollectionEvent updates#5067grolmus wants to merge 3 commits into
Conversation
Load the pre-update Collection with its featuredAsset, assets, channels, parent and translations so the previousEntity exposed on `updated` CollectionEvents is usable for diffing — a bare load left translations and the other relations empty. Consolidates the redundant channel-guard load, documents the new field with @SInCE 3.8.0, and adds e2e coverage asserting the previous name is carried through on update.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Hi Martin, The PR this comes from is rightly labeled T3: systemic. That means it should not be blindly implemented. This is a perfect case in fact, because we should not be ad-hoc solving this for a single event. We need to step back and think about the general problem of "how do we convey the specific changes to an entity via the event system?" |
|
@michaelbromley ah u are right. i didnot notice that it is T3 thanks for your check |
3 participants
Summary
Adds a
previousEntitytoCollectionEvent, populated forupdatedevents, so subscribers can diff a collection against its pre-update state. Builds on and supersedes #4402 by @flobacher (their original commit is preserved).Root cause / motivation
CollectionEventonly exposed the post-update entity, so a subscriber wanting to react to what changed on a collection update had no access to the previous values.The original PR (#4402) added
previousEntitybut loaded it via a baregetEntityOrThrowwith no relations, leavingtranslations(and the other relations) empty — so the previous snapshot was not actually usable for diffing. It also introduced a second collection load alongside the existing active-channel guard.Change
collection-event.ts— newpreviousEntity?: Collectionconstructor field, documented with@since 3.8.0. The doc notes its relation set is not identical toentity, so reliable diffing is limited totranslations,featuredAsset,assetsand scalar columns.collection.service.ts—update()loads the pre-update collection once, withrelations: ['featuredAsset', 'assets', 'channels', 'parent', 'translations']. This single load both enforces the active-channel guard (unchanged semantics —findOneInChannelstill filters by channel) and captures the previous snapshot, which is passed to theupdatedCollectionEvent. The redundant second load is removed.productVariants/childrenare deliberately excluded (potentially huge).translatableSaver.updatebuilds a brand-new entity for the save, sopreviousEntityis a detached pre-update snapshot in all update paths (including filter changes).Test plan
Automated — extends
entity-event-update-state.e2e-spec.ts(the suite that already covers post-update state for Promotion/ShippingMethod/Administrator/PaymentMethod events) with aCollectionEventcase: update a collection's name, assertevent.type === 'updated',event.entityhas the new name,event.previousEntityhas the old name, and that a non-translation relation (channels,parent) is loaded on the snapshot (guards against silently dropping a relation).Verified against a freshly-built core:
Without the fix,
event.previousEntityis undefined and the new assertions fail.Closes #4402
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.